Skip to content

Refactor PLM to use the canonical Learning Mode analyzer - #760

Open
Richie Gomez (richiemsft) wants to merge 5 commits into
mainfrom
refactor-plm-canonical-analyzer
Open

Refactor PLM to use the canonical Learning Mode analyzer#760
Richie Gomez (richiemsft) wants to merge 5 commits into
mainfrom
refactor-plm-canonical-analyzer

Conversation

@richiemsft

@richiemsft Richie Gomez (richiemsft) commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Refactors plm.exe into a mode-neutral legacy WPR helper that delegates sealed-ETL decoding to learning_mode_windows::EtlDenialAnalyzer.

  • removes the duplicate EvtQuery / EvtRender analyzer;
  • supports the canonical block and allow provider vocabulary merged in Consolidate Learning Mode analyzer coverage #759;
  • emits atomic canonical denials.json output and a structured stop result;
  • lets callers select the exact wpr -stop ETL destination with --trace-output;
  • forwards the audited workload exit code into the canonical denials summary;
  • pins both Kernel-General (learningModeLogging) and PermissiveLearningMode providers in the embedded WPR profile;
  • temporarily adapts canonical file/capability denials into the existing adjusted-config generator;
  • refuses incomplete policy generation when analysis is truncated;
  • preserves the legacy current-directory exclusion for compatibility-generated policies;
  • validates output collisions before capture.

The branch is rebased onto main after #759 merged. The rebase had no textual conflicts; semantic compatibility with the merged analyzer was reviewed and corrected before updating this PR.

🔗 References

🔍 Validation

  • cargo test -p plm --all-targets — 133 passed, 1 ignored existing diagnostic-fixture test.
  • Targeted wxc audit stop-argument test.
  • cargo clippy -p plm -p wxc --all-targets -- -D warnings.
  • cargo fmt --all -- --check.
  • Complete local diff review against merged Consolidate Learning Mode analyzer coverage #759.
  • git diff --check origin/main...HEAD.

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task

Copilot AI balanced review requested due to automatic review settings August 6, 2026 17:24
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Forward audit workload exit codes, avoid policy previews from truncated analysis, and retain the legacy current-directory exclusion in the temporary adjusted-config adapter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Copilot AI review requested due to automatic review settings August 7, 2026 21:12
@richiemsft
Richie Gomez (richiemsft) force-pushed the refactor-plm-canonical-analyzer branch from 337fa51 to a1bf155 Compare August 7, 2026 21:12
Store newly added PLM files with repository-standard LF endings so whitespace validation reports the actual content cleanly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
@richiemsft
Richie Gomez (richiemsft) marked this pull request as ready for review August 7, 2026 21:14
@richiemsft
Richie Gomez (richiemsft) requested a review from a team as a code owner August 7, 2026 21:14
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Comment thread src/host/plm/src/stop.rs
Copilot AI review requested due to automatic review settings August 7, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/host/plm/src/analysis.rs:90

  • This conversion no longer applies the legacy AppPath-based self-access filter. The downstream filter only compares against bin_path, but wxc --audit invokes plm stop without --bin-path (src/core/wxc/src/main.rs:1331), so it defaults to the plm executable directory and cannot identify the audited workload's executable. Self-open events that the removed access_failure decoder dropped will therefore be persisted as unnecessary filesystem grants. Preserve this filtering in the canonical decoder before AppPath is discarded, or carry the needed metadata into this adapter.
                    events.push(LearningModeAccessEvent {
                        time_created: chrono::Utc::now(),
                        process_id: denial.pid,
                        thread_id: 0,
                        file_path: denial.resource.clone(),
                        access_mask,
                    });

src/host/plm/src/analysis.rs:70

  • The removed parser rejected control characters and Windows wildcard characters before producing policy entries, but this adapter now checks only the drive prefix and current directory. The canonical analyzer accepts absolute paths without that filename validation, and config::normalize_path also does not reject these characters, so a denial such as C:\data\*.txt can now be persisted into an adjusted policy. Restore the legacy path-shape check before creating the access event.
                if !is_local_drive_path(&denial.resource)
                    || is_current_directory_path(&denial.resource, current_directory)
                {

Comment thread src/host/plm/src/analysis.rs
Normalize non-verbatim Win32 output aliases during collision checks and keep metadata-only READ_CONTROL denials from producing content-access grants.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Copilot AI review requested due to automatic review settings August 7, 2026 22:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/host/plm/src/analysis.rs:72

  • These canonical file denials are promoted without the legacy looks_like_valid_path check, which rejected control characters and Win32 wildcard characters. An attempted path such as C:\foo* can therefore reach readwritePaths/readonlyPaths, producing a policy entry that the old generator intentionally excluded. Retain that validation in the compatibility adapter.
                    events.push(LearningModeAccessEvent {
                        time_created: chrono::Utc::now(),
                        process_id: denial.pid,
                        thread_id: 0,
                        file_path: denial.resource.clone(),
                        access_mask,

src/host/plm/src/analysis.rs:69

  • The compatibility adapter drops the legacy per-event self-access filter. The removed decoder compared each denied object with that event's application path, but DeniedResource carries no application path and wxc's audit stop call does not pass --bin-path; consequently, accesses to the workload executable can now be promoted into the generated policy. Preserve enough event attribution to apply the old self-access exclusion before creating these compatibility events.
    for denial in denials {
        match denial.resource_type {
            ResourceType::File => {
                if !is_local_drive_path(&denial.resource)
                    || is_current_directory_path(&denial.resource, current_directory)

Comment thread src/host/plm/src/stop.rs
Compare existing outputs by filesystem identity, fail closed when identity cannot be examined, normalize default-stream aliases, and preserve canonical workload self-access filtering.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Copilot AI review requested due to automatic review settings August 7, 2026 23:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/backends/learning_mode/windows/src/extractors.rs:226

  • This self-access check drops different files that share the same volume-relative path. For example, ObjectName = D:\Tools\app.exe and AppPath = C:\Tools\app.exe both reduce to \Tools\app.exe, so the real denial on D: disappears from both canonical captureDenials output and the compatibility policy. When both inputs are DOS paths, compare their full paths (including drive); use the volume-relative fallback only when a device-form path prevents that comparison.
        volume_relative_path(object_name),
        volume_relative_path(app_path),
    ) {
        (Some(object_relative), Some(app_relative)) => {
            !object_relative.is_empty() && object_relative.eq_ignore_ascii_case(app_relative)

Comment thread src/host/plm/src/stop.rs
Comment on lines +395 to +396
let key = normalize_win32_components(&key, !is_verbatim);
key.to_ascii_lowercase()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants